Fix performance issue in liveness checking #151376

chenyukang · 2026-01-19T17:09:17Z

r? @ghost

from #150955 (comment)

rustbot · 2026-01-19T17:09:20Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

chenyukang · 2026-01-19T17:09:42Z

@bors try @rust-timer queue

Fix performance issue in liveness checking

lqd

Cool. r=me with green perf

View changes since this review

rust-bors · 2026-01-19T19:31:15Z

☀️ Try build successful (CI)
Build commit: 3a8fd6c (3a8fd6ccca553ed0605e839cc9eac17a3aa80166, parent: d940e56841ddcc05671ead99290e35ff2e98369f)

rust-timer · 2026-01-19T21:26:27Z

Finished benchmarking commit (3a8fd6c): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.2%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 3.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.9%	[3.9%, 3.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -2.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 473.058s -> 474.35s (0.27%)
Artifact size: 383.29 MiB -> 383.32 MiB (0.01%)

chenyukang · 2026-01-20T02:47:00Z

@bors r=lqd

rust-bors · 2026-01-20T02:47:03Z

📌 Commit 54f9a44 has been approved by lqd

It is now in the queue for this repository.

lqd · 2026-01-20T06:53:45Z

It didn’t fix the issue you were trying to fix, right?

chenyukang · 2026-01-20T08:21:40Z

It didn’t fix the issue you were trying to fix, right?

seems that PR #150955 don't have that big performance regression:
#150955 (comment)

lqd · 2026-01-20T08:23:16Z

It doesn't indeed.

lqd · 2026-01-20T08:26:56Z

In any case, this new code is in a good spot now (but it's also not clear that it was causing much in the previous place) so let's land it.

panstromek · 2026-01-20T14:21:58Z

compiler/rustc_mir_transform/src/liveness.rs

+                // By convention, underscore-prefixed bindings are explicitly allowed to be unused.
+                if name.as_str().starts_with('_') {
+                    continue;
+                }


This is inside the loop, so this lookup can be done multiple times after this change, right? Could that be the issue?

For positive cases we can also just break, too.

Suggested change

// By convention, underscore-prefixed bindings are explicitly allowed to be unused.

if name.as_str().starts_with('_') {

continue;

}

// By convention, underscore-prefixed bindings are explicitly allowed to be unused.

if name.as_str().starts_with('_') {

break;

}

I saw that but the only way this would matter is if the benchmarks code emits unused warnings today, and we'd be thus benchmarking diagnostics?

Hmm, right. I guess I'm just perplexed by the results. For code that never emitted these warnings, this PR should be strictly less work, because they never reach this branch anyway, but instead it's a regression. That doesn't make any sense to me. At least it should undo the previous regression in unicode-normalization, but those benchmarks didn't move above the treshold, or slightly regressed, if anything (I'm assuming unicode-normalization doesn't emit unused-assignemnts).

were you able to check @chenyukang? break makes sense to us, but the results are also strange: not knowing whether they impact benchmarks unexpectedly due to diagnostics.

yeah, seems better, updated the code.

what did you learn, are we unexpectedly looking at codepaths that are only exercised through diagnostics emitted by the benchmarks?

break seems more efficient in theory, but no performance improvement in practice, i think diagnostics emitting is general not in hotpath, don't understand why #150955 (comment) happened, this PR should make sure name.as_str().starts_with('_') only executed when there is an error, but #151376 (comment) makes no change.

@ghost

Fix performance issue in liveness checking r? @ghost from #150955 (comment)

rust-bors · 2026-01-20T20:45:00Z

💔 Test for 399f6ba failed: CI. Failed job:

auto - x86_64-gnu-aux (web logs, enhanced plaintext logs)

chenyukang · 2026-01-27T06:51:16Z

There error is cannot open file descriptor 3 from the jobserver environment variable value: Bad file descriptor (os error 9), seems unrelated to my change.

@bors try jobs=x86_64-gnu-aux

Fix performance issue in liveness checking try-job: x86_64-gnu-aux

rust-bors · 2026-01-27T09:11:59Z

☀️ Try build successful (CI)
Build commit: b548f68 (b548f68481ab156f026c547e926c00a3bcf048f3, parent: ebf13cca58b551b83133d4895e123f7d1e795111)

chenyukang · 2026-01-30T09:21:05Z

anyway, let's run a perf again

@bors try @rust-timer queue

Fix performance issue in liveness checking

rust-bors · 2026-01-30T11:32:35Z

☀️ Try build successful (CI)
Build commit: 98437b3 (98437b3d28233c3a5d7a8d07f1459ed8ea5b1068, parent: ef2657cbaf6885b71e5ffd277ffe72a2616c0a7c)

rust-timer · 2026-01-30T12:12:49Z

Finished benchmarking commit (98437b3): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 545.612s -> 476.413s (-12.68%)
Artifact size: 397.86 MiB -> 397.86 MiB (0.00%)

lqd · 2026-01-30T12:18:37Z

Great, no noise, @bors r+ rollup

rust-bors · 2026-01-30T12:18:40Z

📌 Commit d2aa64f has been approved by lqd

It is now in the queue for this repository.

…uwer Rollup of 4 pull requests Successful merges: - #151376 (Fix performance issue in liveness checking) - #151851 (Remove redundant `IntoQueryParam` calls from query plumbing) - #151854 (Show break type expectation cause for let-else) - #151859 (Disable append-elements.rs test with debug assertions)

@ghost

Rollup merge of #151376 - chenyukang:yukang-fix-perf, r=lqd Fix performance issue in liveness checking r? @ghost from #150955 (comment)

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 19, 2026